-
-
Notifications
You must be signed in to change notification settings - Fork 43
✨ Collect clifford blocks #885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Collect clifford blocks #885
Conversation
Signed-off-by: Jannik Pflieger <[email protected]>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this initial PR 🙏🏼
I just went through everything and left a couple of inline comments. This still needs a bit of work before it is ready to be merged.
Don't hesitate to ask if you should have any questions.
@jannikpflieger any updates on this? it's been three weeks since the review above 😌 |
@burgholzer still doing it, since I was on vacation + exam, but I'm on it! I'm back since yesterday evening :D |
…-core-cliffordblockfinder into collectCliffordBlocks
…llect blocks, changed isClifford function, added docstring
…-core-cliffordblockfinder into collectCliffordBlocks
…-core-cliffordblockfinder into collectCliffordBlocks
…-core-cliffordblockfinder into collectCliffordBlocks
…-core-cliffordblockfinder into collectCliffordBlocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for another iteration here. To be fair and honest, the implementation here still looks way to complicated compared to what I had in mind. At least, it seems to be doing the correct thing now though. So I'd propose the following: I left a couple of minor inline comments on the tests to clarify some things and improve the clarity of the tests. Once addressed, let's get this merged and close this chapter.
…-core-cliffordblockfinder into collectCliffordBlocks
…-core-cliffordblockfinder into collectCliffordBlocks
…-core-cliffordblockfinder into collectCliffordBlocks
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a couple of touch ups as well as a changelog entry.
Let's get this in 🚀
Thanks for the contribution! Much appreciated 🙏🏼
Description
Added the ability to separate the collection of Clifford and non Clifford Blocks by adding a new function parameter. For that a isCliffordCheck was added as well. The isClifford check is very rough but includes all common Clifford gates. However, it does not include rotation gates that with the right angle could be considered Clifford. It does not include custom gates that could be Clifford as well.
The separation is done by finalizing a block when we iterate over a non-Clifford Block. When we want to collect Clifford and non Clifford gates separately then the function handles non-Clifford gates as "cannot be processed" and the canProcess variable is set to false. This ensures that we still have large CliffordBlocks but we have only single non CliffordBlocks. Essentially separating the non-CliffordBlocks from all Clifford and other non-CliffordBlocks.
Checklist: